Skip to content

Replace finders inheritance hierarchy with composition - #16154

Open
borinquenkid wants to merge 16 commits into
8.1.xfrom
test/document-datamapping-core-finders
Open

Replace finders inheritance hierarchy with composition#16154
borinquenkid wants to merge 16 commits into
8.1.xfrom
test/document-datamapping-core-finders

Conversation

@borinquenkid

Copy link
Copy Markdown
Member

Summary

  • Replaces the 4-level AbstractFinder -> DynamicFinder -> AbstractFindByFinder -> concrete finder inheritance chain in org.grails.datastore.gorm.finders (grails-datamapping-core) and the parallel hierarchy in org.grails.gorm.rx.finders (grails-datamapping-rx) with flat classes configured via static factories, composing a shared DynamicFinder/FinderGrammar instead of extending it.
  • Fixes a confirmed dead-code bug in the old FindOrSaveByFinder: its own "construct from Equal expressions on null result" branch could never execute because the superclass (FindOrCreateByFinder) already performed construction-and-conditional-save before it. The new design has exactly one implementation of that logic (SingleResultFinder#constructFromEqualExpressions), shared by findOrCreateBy/findOrSaveBy, so this class of bug can't recur.
  • A TCK backend spot-check (grails-datamapping-core-test) caught a real regression introduced mid-refactor - a dropped invoke(Class, String, DetachedCriteria, Object[]) overload needed by AbstractDetachedCriteria#methodMissing's dynamic Groovy dispatch - which is fixed and covered by dedicated unit tests on all 6 affected classes (3 core, 3 rx).
  • Every other pre-existing behavior/quirk is preserved and re-asserted in tests rather than silently fixed, including: the And/Or literal-split collision, the empty-property-name-on-operator-collision crash, the sync/rx findAllBy .distinct() inconsistency, and rx's findOrCreateBy/findOrSaveBy missing the Or/comparison-operator validation the sync side has.
  • Follow-up commits address IntelliJ warnings surfaced across the touched classes (raw types, unused parameters/constructors turning out to be reflectively-used false positives, redundant conditions, a couple of small dedups), each verified against the full module test suites.

Test plan

  • :grails-datamapping-core:test - full suite green
  • :grails-datamapping-rx:test - full suite green
  • :grails-datamapping-core-test:test (TCK-backed, exercises real GORM dynamic-finder dispatch end-to-end) - full suite green
  • :grails-datamapping-core:codeStyle / :grails-datamapping-rx:codeStyle - clean

🤖 Generated with Claude Code

borinquenkid and others added 14 commits August 15, 2026 13:59
The 4-level DynamicFinder inheritance chain (AbstractFinder ->
DynamicFinder -> AbstractFindByFinder -> concrete finders) made
FindOrSaveByFinder carry dead code: its own copy of the
construct-from-Equal-expressions logic could never run because
FindOrCreateByFinder's superclass method already handled it. The
FindByBooleanFinder/FindAllByBooleanFinder pair duplicated the same
pattern-swap override for the same structural reason.

Replace the hierarchy with flat classes (SingleResultFinder,
ListResultFinder, CountFinder, ListOrderByFinder) that compose a
DynamicFinder grammar instance and are configured via static
factories, in both grails-datamapping-core and its rx mirror. The
construct-from-Equal-expressions logic and the boolean-clause
handling now exist in exactly one place each.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Fix a dangling Javadoc cross-reference in SingleResultFinderSpec to
  a nonexistent "field-ordering note".
- Add an explicit 0 * query.projections() assertion in
  RxListResultFinderSpec so the "no distinct()" quirk documented in
  RxListResultFinder's class Javadoc is self-verifying rather than
  incidental.
- Add a createFinderInvocation-level test for the And/Or literal-split
  quirk in DynamicFinderSpec; previously only buildMatchSpec (the
  compile-time matcher) exercised it, leaving the actual runtime path
  finders call unverified.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FinderMethod declares invoke(Class, String, ...) with raw Class/
Closure parameters (suppressed at the interface level), so every
override necessarily reintroduces the same raw-type warning at its
own call site - IntelliJ flags each override individually, and
ListOrderByFinder already suppresses per-method for the same reason.
Apply the same @SuppressWarnings("rawtypes") to CountFinder's three
invoke overloads, including the DetachedCriteria one, which stays raw
to match DynamicFinderInvocation.setDetachedCriteria's existing raw
parameter type rather than diverging from it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- operatorPatterns/operators fields are only ever assigned in the
  constructor - make them final.
- Diamond operator for the methodExpressions map initializer.
- Merge two identical-body SecurityException/NoSuchMethodException
  catch branches into multi-catch, in both the static initializer and
  registerNewMethodExpression.
- Suppress the intentionally-ignored Matcher.find() result in
  createFinderInvocation (isMethodMatch already guarantees the match
  succeeds before a finder's invoke() is ever called).
- buildMatchSpec: the local pattern "(prefix)([A-Z]\w*)" has no
  alternation, so group 2 is guaranteed non-null whenever find()
  succeeds - drop the always-true querySequence != null conjunct, and
  collapse the now childless outer if into the inner one.
- Drop two self-reassignments (x = getInitializedExpression(x, args))
  now that the helper is confirmed to always mutate-and-return the
  same reference; and delete a soloArgs array that was populated but
  never read.
- Extract the ~25-line fetch/cache argument-map handling duplicated
  verbatim between both populateArgumentsForCriteria overloads into a
  shared applyFetchAndCacheArguments helper, parameterized over the
  join/cache operations since Query and BuildableCriteria don't share
  a supertype exposing them. Convert the surviving sort-map branches
  to pattern variables, dropping an unused `value` local along the
  way.
- configureQueryWithArguments: fold the instanceof check and cast into
  a single pattern-variable condition.
- Drop the redundant Iterable cast on methodExpressions.keySet() and
  replace criteria.remove(0) with removeFirst().
- Leave targetClass's unused-parameter warning suppressed rather than
  removing the parameter (call-site compatibility with
  grails-data-hibernate5/7 and grails-data-mongodb) and leave the
  Class<? extends Object> wildcard alone (documented GROOVY-9460
  workaround already noted in the existing TODO).

Verified via the full grails-datamapping-core and
grails-datamapping-core-test suites - no behavior change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Checked: GROOVY-9460 (Class<D> rejected against a Class<?> parameter
under static compilation) was fixed in Groovy 2.5.16/3.0.6/4.0.0-alpha-1.
This project is on Groovy 5.0.x, well past the fix, so the
Class<? extends Object> workaround is obsolete - simplify to Class<?>.

Verified against every real call site, including the @CompileStatic
grails.gorm.DetachedCriteria#list, which passes a Class<T> field
(exactly the pattern the bug affected): grails-datamapping-core,
grails-datamapping-rx, and grails-data-hibernate5-core all compile
clean, and the core test suite is green.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- getInitializedExpression's return value is now unused at both call
  sites (the last commit dropped the self-reassignments) - change it
  to void and drop the stale "@return"/removed-IsNull-path Javadoc
  that no longer matched the commented-out code.
- Both populateArgumentsForCriteria overloads wrapped their sort-object
  instanceof chain in a redundant "if (sortObject != null)" guard -
  instanceof already returns false for null, so the wrapper is
  eliminable without changing behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both the "select"/LAZY-named-mode if-branch and the final fallback
returned FetchType.LAZY, so the if condition was a no-op guard around
an outcome identical to just falling through - drop it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
javaClass/methodName/arguments/expressions/criteria/operator are only
ever set in the constructor and have no setters - make them final.
detachedCriteria keeps its mutable setter, so it stays as-is.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
createFinderInvocation's Class/Closure parameters are raw to match
FinderMethod's existing raw signature (finder invoke() overloads
delegate straight through). Suppress at the interface level, same as
FinderMethod itself already does.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- match.find() result is intentionally ignored - isMethodMatch already
  guarantees the match succeeds before invoke() is ever called, same
  as DynamicFinder#createFinderInvocation.
- new LinkedHashMap((Map) arguments[0]) is an unchecked copy-constructor
  call on a raw Map, needed since FinderMethod's invoke() signature is
  itself raw.

Both suppressed on the enclosing invoke() method, alongside the
existing rawtypes suppression.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Raw Class/Closure/DetachedCriteria on the three invoke overloads,
  same pattern as CountFinder/SingleResultFinder.
- findAllByBoolean(MappingContext) had no caller anywhere - it mirrors
  the Datastore/MappingContext factory-pair pattern every finder class
  uses (the MappingContext overload exists for unit testing without a
  full Datastore, as findAllBy(MappingContext) already is), so add the
  missing test instead of removing the factory.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Raw Class/Closure/DetachedCriteria on the three invoke overloads.
- constructFromEqualExpressions' Map.put on the raw HashMap is
  "unchecked", a separate category from the "rawtypes" it already
  suppressed.
- findByBoolean/findOrCreateBy/findOrSaveBy(MappingContext) had no
  callers, same Datastore/MappingContext factory-pair pattern as
  ListResultFinder - add the missing stateless-mode tests (mirroring
  findBy(MappingContext)'s existing one) instead of removing them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every flagged constructor is genuinely exercised, just never via a
literal "new Xxx(...)" expression for these particular classes:
DynamicFinder's registry invokes the (Class, String) constructor
reflectively via Constructor.newInstance, and MethodExpressionSpec's
existing round-trip tests invoke both shapes reflectively via
Class.getConstructor(...).newInstance(...) - so IntelliJ's static
usage tracker can't see either call site. Suppress rather than add
redundant tests, since coverage already exists.

Also suppress the base constructor's unused targetClass parameter -
the field it feeds is already @deprecated and unused by design, kept
only so every subclass can offer the (Class, String) shape uniformly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
convertArguments and convertArgumentsForProp each had their own copy
of "fall back to the identity property when getPropertyByName returns
null" - extract a shared resolveProperty helper instead. This also
lets convertArgumentsForProp drop its now-unused PersistentEntity/
propertyName parameters, and removes NotInList#convertArguments'
redundant self-assigned locals along the way.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 15, 2026 21:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request refactors the GORM dynamic-finder implementations (sync and Rx) from deep inheritance hierarchies into flat, factory-configured finder classes that compose a shared DynamicFinder/FinderGrammar. This modernizes the design, reduces duplication/drift risk between finder variants, and adds dedicated unit test coverage for the new entry points (including reflective DetachedCriteria overloads used by Groovy dispatch).

Changes:

  • Introduces composed finder implementations: SingleResultFinder / ListResultFinder / CountFinder (core) and RxSingleResultFinder / RxListResultFinder / RxCountFinder (rx).
  • Updates enhancer/static API wiring (GormEnhancer, RxGormStaticApi) to register the new factories instead of instantiating the old subclasses.
  • Adds/updates tests to exercise the real public invoke(...) seams (including invoke(Class, String, DetachedCriteria, Object[])).

Reviewed changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/RxSingleResultFinder.groovy New composed Rx “single result” dynamic finder (findBy/findByBoolean/findOrCreateBy/findOrSaveBy).
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/RxListResultFinder.groovy New composed Rx “list result” dynamic finder (findAllBy/findAllByBoolean).
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/RxCountFinder.groovy New composed Rx countBy finder reusing core CountFinder.applyCriteriaAndCount.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/api/RxGormStaticApi.groovy Rewires Rx dynamic-finder registration to new finder factories.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxSingleResultFinderSpec.groovy New spec covering the new Rx single-result finder entry points.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxListResultFinderSpec.groovy New spec covering the new Rx list-result finder entry points.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxCountFinderSpec.groovy Updates tests for the new Rx countBy finder entry points.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindByFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindByBooleanFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindAllByFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindAllByBooleanFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindOrCreateByFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/FindOrSaveByFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/finders/CountByFinder.groovy Removed legacy inheritance-based Rx finder.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/FindByFinderSpec.groovy Removed legacy Rx finder spec.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/FindByBooleanFinderSpec.groovy Removed legacy Rx finder spec.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/FindAllByFinderSpec.groovy Removed legacy Rx finder spec.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/FindAllByBooleanFinderSpec.groovy Removed legacy Rx finder spec.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/FindOrCreateByFinderSpec.groovy Removed legacy Rx finder spec.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/FindOrSaveByFinderSpec.groovy Removed legacy Rx finder spec.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/SingleResultFinder.java New composed sync “single result” dynamic finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/ListResultFinder.java New composed sync “list result” dynamic finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/CountFinder.java New composed sync countBy finder (with reusable helper for Rx).
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FinderSupport.java Replaces AbstractFinder with a shared static session-execution helper.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FinderGrammar.java Introduces a composition seam implemented by DynamicFinder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java Refactors DynamicFinder into a composable grammar/utility class (not a base finder).
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/MethodExpression.java Refactors argument conversion/property resolution and documents reflective constructors.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinderInvocation.java Makes invocation fields immutable (final) aside from detached criteria.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/ListOrderByFinder.java Refactors listOrderBy finder to implement FinderMethod directly (no base class).
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEnhancer.groovy Rewires sync dynamic-finder registration to new finder factories.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/SingleResultFinderSpec.groovy New spec for composed sync single-result finder.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/ListResultFinderSpec.groovy New spec for composed sync list-result finder.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/CountFinderSpec.groovy New spec for composed sync countBy finder behavior.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/ListOrderByFinderSpec.groovy New spec for refactored listOrderBy finder.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/FinderSupportSpec.groovy New spec for “stateless mode” guard behavior.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/DynamicFinderInvocationSpec.groovy New spec for invocation value-object behavior.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/MethodExpressionSpec.groovy New spec to comprehensively cover method-expression operators and conversions.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/finders/FinderTestEntity.groovy New shared fixture for finder-package unit tests.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindByFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindByBooleanFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindAllByFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindAllByBooleanFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindOrCreateByFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/FindOrSaveByFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/CountByFinder.java Removed legacy inheritance-based sync finder.
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/AbstractFindByFinder.java Removed legacy intermediate base class.
Suppressed comments (4)

grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxSingleResultFinderSpec.groovy:128

  • Same issue as the non-detached-criteria case: this test stubs singleResult() to return an entity instance, but RxGORM expects RxQuery.singleResult() to return an Observable. Using an Observable.just(...) stub ensures the test matches production behavior.
    grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxCountFinderSpec.groovy:75
  • RxCountFinder ultimately calls singleResult() on an Rx query, which is expected to be reactive (RxQuery.singleResult() returns an Observable). Stubbing a raw Long here doesn’t reflect the real contract and can hide a type mismatch in RxGormStaticApi.methodMissing.

This issue also appears on line 96 of the same file.
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxCountFinderSpec.groovy:97

  • Same return-type issue in this test: singleResult() should be stubbed as an Observable (per RxQuery.singleResult()), and the assertion should consume the observable, otherwise the test can pass while production code returns an Observable<Number>.
    grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxSingleResultFinderSpec.groovy:179
  • findByBoolean is invoked from RxGormStaticApi.methodMissing (which returns an Observable), so stubbing singleResult() with a raw entity here doesn’t match the expected reactive return type (RxQuery.singleResult()). This test should use an Observable stub and assert by consuming it.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@bito-code-review

Copy link
Copy Markdown

The observation regarding the test stubbing is correct. In RxGORM, reactive query methods return an Observable (or similar reactive type), and stubbing Query.singleResult() to return a domain instance directly bypasses the reactive contract. This can indeed mask integration issues where the code expects a reactive stream rather than a synchronous result.

To address this, the test should be updated to return an Observable that emits the domain instance, ensuring the test exercises the reactive flow as it would in a real application.

@codecov

codecov Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.13043% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.3114%. Comparing base (506228b) to head (63ba0e9).
⚠️ Report is 277 commits behind head on 8.1.x.

Files with missing lines Patch % Lines
...g/grails/datastore/gorm/finders/DynamicFinder.java 97.5309% 1 Missing and 1 partial ⚠️
...grails/gorm/rx/finders/RxSingleResultFinder.groovy 98.1132% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.1.x     #16154        +/-   ##
==================================================
+ Coverage     52.8254%   53.3114%   +0.4860%     
- Complexity      18871      19456       +585     
==================================================
  Files            2079       2071         -8     
  Lines           97207      98975      +1768     
  Branches        16873      17355       +482     
==================================================
+ Hits            51350      52765      +1415     
- Misses          38446      38681       +235     
- Partials         7411       7529       +118     
Files with missing lines Coverage Δ
...oovy/org/grails/datastore/gorm/GormEnhancer.groovy 63.7405% <100.0000%> (ø)
...org/grails/datastore/gorm/finders/CountFinder.java 100.0000% <100.0000%> (ø)
...atastore/gorm/finders/DynamicFinderInvocation.java 100.0000% <ø> (ø)
...g/grails/datastore/gorm/finders/FinderSupport.java 57.1429% <100.0000%> (ø)
...ails/datastore/gorm/finders/ListOrderByFinder.java 92.3077% <100.0000%> (+7.6923%) ⬆️
...rails/datastore/gorm/finders/ListResultFinder.java 100.0000% <100.0000%> (ø)
...rails/datastore/gorm/finders/MethodExpression.java 97.2678% <100.0000%> (+34.6362%) ⬆️
...ils/datastore/gorm/finders/SingleResultFinder.java 100.0000% <100.0000%> (ø)
...oovy/org/grails/gorm/rx/api/RxGormStaticApi.groovy 92.8177% <100.0000%> (ø)
...vy/org/grails/gorm/rx/finders/RxCountFinder.groovy 100.0000% <100.0000%> (ø)
... and 3 more

... and 85 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

borinquenkid and others added 2 commits August 15, 2026 18:09
…s Observable

query.singleResult() returns Observable<T> in production (RxQuery#singleResult),
and the plain findBy/findByBoolean/countBy paths pass that value straight through
without unwrapping it. Five tests stubbed singleResult() with a raw entity/Long
instead, exercising a shape that can never occur in production and silently
skipping the unwrap step real callers must do. Flagged by automated PR review on
#16154.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ug in RxSingleResultFinder

Adds tests for setPattern delegation, the null-detachedCriteria branch of the
reflective 4-arg invoke() overload, the ConversionException->MissingMethodException
mapping, DynamicFinder's buildMatchSpec/configureQueryWithArguments/
populateArgumentsForCriteria branches, and ListOrderByFinder's Map-argument
handling, across the six composed finder classes (sync + rx).

Also fixes a genuine latent bug surfaced while closing these gaps:
RxSingleResultFinder's findOrCreateBy*/findOrSaveBy* empty-result path ran
inside a spawned Thread with no exception handling, so a thrown
MissingMethodException (e.g. from a non-Equal expression) killed the thread
silently instead of reaching the Observable's error channel - any blocking
caller would hang forever. Wrapped the thread body so failures now propagate
via Subscriber#onError, and added a test proving the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@testlens-app

testlens-app Bot commented Aug 16, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 63ba0e9
▶️ Tests: 64329 executed
⚪️ Checks: 77/77 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants